Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding rpi support for prometheus + some supplementals #383

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Simoliv
Copy link

@Simoliv Simoliv commented Nov 3, 2022

Decided I want prometheus support in RPi thing.

Added docker-compose to run whole prometheus stack on that rpi. For me its sufficient to have stats when I am at home.

Added systemd file to run the hoymiles thing as a service on rpi.

Copy link
Collaborator

@DanielR92 DanielR92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 'tools/rpi/optional-requirements.txt' you write "prometheus_client", is here no version available?

I cant test it now, but it shows clear for me.
No abnormalities discovered.

I haven't tested it, may have to wait a bit until someone can verify that. But at first glance it looks to me like a clean extension with Prometheus for Grafana.

@Simoliv
Copy link
Author

Simoliv commented Nov 3, 2022

I didnt put version, because I tested it with at least 2 different versions. I dont rely on any special stuff in that prometheus-client library, just core functionality, so I would let pip decide which version suits best to all the other requirements. Thats why I didnt put any specific version. Not like with CUDA and/or tensorflow stuff and such .. ;)

@stefan123t
Copy link
Collaborator

Except for the time-to-sleep you factored out in tools/rpi/main.py it looks quite readable to me.

@Simoliv
Copy link
Author

Simoliv commented Nov 4, 2022

Except for the time-to-sleep you factored out in tools/rpi/main.py it looks quite readable to me.

Thats right, might be a relict from testing. Left by mistake. I will try to put it again.

@Simoliv
Copy link
Author

Simoliv commented Nov 4, 2022

Except for the time-to-sleep you factored out in tools/rpi/main.py it looks quite readable to me.

Thats right, might be a relict from testing. Left by mistake. I will try to put it again.

While doesnt seem to be required when looking at it closely .. Just removed variable and put the code from it directly where the variable was used. Please dont ask me why I did .. Dont remember anymore

@fred777
Copy link

fred777 commented Nov 5, 2022

Except for the time-to-sleep you factored out in tools/rpi/main.py it looks quite readable to me.

Thats right, might be a relict from testing. Left by mistake. I will try to put it again.

While doesnt seem to be required when looking at it closely .. Just removed variable and put the code from it directly where the variable was used. Please dont ask me why I did .. Dont remember anymore

Ok, but this way you re-introduced a bug that could lead to negative sleep time and thus a crash of the application ;-)

#371

So before creating a merge request, it is highly advisable to merge the - most likely updated - main branch into your feature branch to make sure you didn't miss anything.

@Simoliv
Copy link
Author

Simoliv commented Nov 7, 2022

Anything I should do now ? Just clarifying responsibilities .. expecting anything from me ?

@stefan123t
Copy link
Collaborator

@Simoliv could you first merge the fix #371 from @fred777 so we can continue with the merge of your PR #383 :)

@lumapu
Copy link
Owner

lumapu commented Mar 26, 2023

does anybody maintain this feature?

@DanielR92
Copy link
Collaborator

I am not sure, from my side.... no.

@Simoliv
Copy link
Author

Simoliv commented Mar 27, 2023 via email

@Simoliv
Copy link
Author

Simoliv commented Mar 29, 2023

I tried to merge latest stuff into my own fork, but at least on my RPI4 ist not running after merging everything .. so I cannot verify its running with this latest stuff .. What to do ?

@Dreistein75
Copy link

feature looks nice. I'm looking forward to that!!

@Simoliv
Copy link
Author

Simoliv commented Apr 25, 2023

Guys, wanted to come back to this after a while now. Anything I can do or are you still wating for me to do anything ?

Copy link

@fred777 fred777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me so far, and the fix for #371 is included in a sense 😉

But I only looked at the code, did not test anything. So if prometheus stuff is not working, you need to fix it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants